Skip to content

[ci]: gate the public API surface and the published package shape - #1471

Open
sf-tyler-jeong wants to merge 17 commits into
mainfrom
chore/public-api-surface-snapshot
Open

sf-tyler-jeong wants to merge 17 commits into
mainfrom
chore/public-api-surface-snapshot

Conversation

@sf-tyler-jeong

@sf-tyler-jeong sf-tyler-jeong commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Public API surface changes are caught by a person reading a diff. Props, exports, types and the shape a context or hook returns are all contracts a customer builds against, and once one of them slips out it cannot be taken back.

This checks a machine-written snapshot of that surface into the repository and has CI regenerate and compare it on every build. A surface that moves fails the build. An intended change is committed together with the snapshot, so the diff says exactly what moved.

The same idea is applied to the shape of the published package, with publint and @arethetypeswrong/cli. Turning that on surfaced two real defects, fixed here.

Changes

The surface snapshot — api/surface.d.ts

Starting from the entry points in rollup.module-exports.mjs, the generator collects every declaration reachable from them through dist/types — every relative import a declaration carries, including the ones written for their side effect alone, and every <reference path> directive. Package specifiers are not followed: those types belong to their own packages.

The file is keyed by what it records, not by where it came from. A path in a key would make the gate fail on changes it has no opinion about — renaming one internal helper rewrote seven lines, and moving it across directories rewrote forty-six, with no declaration body touched. A gate that fails on those is one reviewers learn to skim.

So the two halves are keyed differently, because they are different contracts:

  • Entry points keep whole-file blocks, keyed by the public subpath. Which names a subpath exports is itself a contract — move a type out of a public module without re-exporting it and the subpath stops exporting it, while a record that only says "this symbol exists somewhere" has nothing to compare.
  • Internal declarations are one block per symbol, keyed by the symbol. The key is derived from what the declaration says and from what it refers to — imported types and types declared beside it alike — resolved to a fixed point that ends when the keys stop moving. A reference names the symbol it reaches for rather than the file holding it, so an unrelated export joining that file moves nothing.

Names are followed to their declaration through aliases, through a binding imported and then exported on, through a namespace, and out to a package, whose specifier is kept since that is as far as this repository can see. Whether a name leaves type-only travels with it, because that is the difference between a consumer constructing a class and failing to.

Blocks are laid out by comparing their keys as strings rather than by a collation, which changes with the shell's locale. The file regenerates byte-identically on any machine, in under two seconds.

The snapshot opens with the list of entry point names. Two entry points that share a source are recorded under one key carrying both names, so dropping one of a pair already shows there — Channel/components/MessageInput and Channel/components/MessageInputWrapper point at the same source today, and removing either rewrites that key. What the list catches on its own is an entry point with no declarations to record: drop lame.all and the one line that moves is its name.

CI regenerates after yarn build and compares with git diff --quiet. It first refuses to run if api/surface.d.ts is untracked, since a deleted file makes the comparison pass silently.

Measured against real src edits and rebuilds, not hand-written fixtures:

change snapshot
rename an internal file 0 lines
change one line of an internal signature 2 lines
add a public export 1 line
a public subpath stops exporting a name 2 lines, both removals

The boundary has a cost, and it is deliberate. Moving a declaration out of a public module shows even when a re-export keeps the subpath exporting it, because the entry block records that module verbatim. That is the same property that lets the snapshot see a subpath lose a name, so the two come together; resolving re-exports into the block instead would expand every entry's closure into it.

Why not an existing tool

Both candidates were installed and run against this package before the generator was written.

@microsoft/api-extractor does this workflow canonically but models one entry point per run: 662ms each here, so about 134 seconds for 202 entry points, plus a config and a committed report for each. Its report also omits types that are referenced but not exported from that entry point — nineteen ae-forgotten-export warnings in the root report alone — so a change to one of them would not show.

tsnapi matches the per-subpath model and is fast, but expects a bundled .d.ts per entry. Against a declaration tree it leaves nineteen of the 203 entries it produces as unresolved relative re-exports, the root . among them.

Neither is a drop-in for a package that ships a declaration tree with 202 subpaths.

The published package shape — yarn package:check

publint reads the manifest and attw resolves every entry point the way a consumer would. Both were run by hand first, and what they turned up is fixed below.

Two rule classes the package already triggered before this change — false-esm and internal-resolution-error — are excluded, so the gate reports regressions instead of requiring the existing baseline to be cleared first. The two entry points excluded by path are untyped by nature: the stylesheet and lame.all.

Two defects it found

The manifest pointed at a declaration file the package never shipped. lame.all is the only entry point whose source is plain JavaScript, so the .replace(/\.tsx?$/, '.d.ts') that derives the declaration path matched nothing and left the .js extension in place. Both exports and typesVersions named types/_externals/lamejs/lame.all.js, which tsc --emitDeclarationOnly never writes — the directory is not in the tarball. publint reports it as an error. The types condition is now omitted when the source emits no declaration. Against the manifest published as 3.19.0, the only differences are that key and its typesVersions entry; the other 203 export entries are unchanged.

A failed build left the manifest unwritten. post_build.js called movePackageJSON() without awaiting it, so its write raced the synchronous work behind it. execSync holds the loop, so a failure in the declaration build ended the process before the continuation ran and dist/package.json was left missing. The steps are sequenced and a failure is now reported explicitly instead of as an unhandled rejection.

Type checking for scripts/

Nothing read the TypeScript this branch adds under scripts/: eslint is scoped to src/**/*.ts*, tsconfig.json includes only src, and vitest strips types without checking them. A separate project rather than widening the shared config: setting allowJs there would reach the build too, since tsconfig.build.json overrides include but inherits compilerOptions. Measured, that alternative grows the declaration emit from 546 to 551 and puts five mock files into the package, so it was rejected. Scoped this way the emit is untouched.

Backward compatibility

No public API changes. Not a line of src/ is touched, and props, exports, types, context return shapes, CSS class names and StringSet keys are all unchanged.

The one manifest difference is the types pointer removed from lame.all, and it changes nothing for a consumer. Measured against a package with the old shape and one with the new, TypeScript reports the same TS7016 and resolves to the same JavaScript file under bundler, node16 and node10 alike — it falls through a types condition that resolves to nothing. A control with a real .d.ts compiles clean, so the comparison does distinguish the cases. What changes is that the manifest no longer names a file that is not in the package, which is what publint and attw read.

Testing

  • yarn lint, yarn typecheck, yarn build, yarn test, yarn api:snapshot and yarn package:check all pass. Every commit on the branch was also checked out on its own and passes lint, tsc, the generator tests and the conformance suite, and reproduces the snapshot it carries
  • The gate has teeth. Removing ui/Toggle from rollup.module-exports.mjs and regenerating drops 63 lines from the snapshot, every one of them a removal, including its entry point line — and git diff --quiet fails

yarn api:conformance — 50 cases

A hand-written fixture can only assert what the person writing it already believed, and one that is wrong in the same direction as the code proves nothing. This suite derives both ends from tools instead: the real compiler emits the declarations, the generator snapshots them, the sources change, and the compiler then type-checks a consumer against the rebuilt package. A case passes when the consumer breaking and the snapshot moving agree — a change a consumer feels has to show, and one it does not feel must not.

Each of the 50 is a shape that slipped past the generator at some point: a name narrowed to type-only along a re-export chain, a binding imported and then exported on under another name, a namespace renamed or repointed, a re-export leaving for a different package, a star asked to carry a default export, a chain passing back through a module it has already been to for a different name, two declarations that read alike but reach for different types, modules that re-export each other in a circle. --deep adds a five-hundred-link alias chain. It is kept out of yarn test, which it would roughly double.

The generator's own tests — 114

Symbol-level extraction can shrink silently: a declaration the splitter fails to recognise leaves no trace, and a smaller snapshot reads as a clean surface. The spec asserts every line of every collected file survives, that keys stay unique, that a block keeps the reference binding each name it uses and none it does not, and that a reference does not move when the file behind it gains an unrelated export. Each guard was confirmed by reverting it and watching the suite go red.

Standing invariants on the committed file: every declaration line of all 294 collected files is present, along with all 725 exported names, 93 @deprecated tags and 201 of the 202 public subpaths — the one left out is vendored JavaScript, which the compiler emits no declarations for. All 730 block keys are unique, and every reference either names a block, a module, or a package.

Reverted to count the failures

Each fix was reverted and the failures counted, then restored and confirmed by hash:

  • entry point header out of renderSnapshot → 1 failure
  • await out of post_build.js → 1 failure
  • null return out of declarationPath → 4 failures
  • keys compared by collation instead of as strings → 1 failure, and the file reorders under a Turkish locale
  • a declaration's co-declared names taken for references → 1 case and 2 tests
  • only the first name of a const a = 1, b = 2; recorded → 2 cases and 3 tests
  • a side-effect import left unfollowed, and an unnamed default recorded as a word → 1 case and 1 test each

Changelogs

  • CI and build scripts only; no change to the published code or types (internal)

The published package exposes 202 subpath entries, and what each one declares
is only visible in the generated .d.ts tree — which the build throws away. So a
change like dropping a context action or turning Promise<void> into void leaves
no trace in the diff: tsc stays green because the library is internally
consistent, and the break surfaces in a customer's build instead of ours.

`yarn api:snapshot` walks the entry points in rollup.module-exports.mjs, follows
re-exports and inline import types through dist/types, and writes the reachable
declarations to api/surface.d.ts. Internal modules that no entry point reaches
are left out — 294 of 546 files survive the filter.

The file is generated, never edited. Regenerating it twice over one build gives
a byte-identical result.
Regenerates the snapshot after the build and compares it with the committed
copy. A mismatch means the public surface moved without anyone recording it, so
the step prints the diff and fails; an intended change passes once
api/surface.d.ts is regenerated and committed alongside the code.

The job name is unchanged, so the existing required check covers this without
touching the ruleset, and `yarn build` already runs here — the step adds no
build time.

It also asserts the snapshot is tracked. Left untracked, `git diff` reports no
change and the gate would pass silently forever.
The snapshot is only as good as the walk that builds it. If following
`export *`, a directory index, or an inline `import()` type ever regresses, the
file shrinks, the diff looks empty, and the gate reports a clean surface while
saying nothing — the failure mode with no symptom.

Splits the walk out of the CLI so it can be driven against a fixture tree and
covers it with fourteen cases: entry mapping, specifier resolution, star and
named re-exports, inline import types, cycles, several entries merging, an entry
whose declaration was never emitted, and the snapshot's format and determinism.
Verified by breaking the walk — dropping inline import tracking fails one case,
dropping index resolution fails two.

vitest looked only under src/, so the include now reaches scripts/ as well. The
generated snapshot is byte-identical to the one already committed.

Two smaller repairs: the workflow now says why a missing api/surface.d.ts
matters instead of leaving a bare pathspec error, and the script warns when src/
is newer than dist/types, since a stale tree snapshots cleanly and the mismatch
would only appear in CI.
The script decided whether to run by comparing process.argv[1] with
import.meta.url, so that it could also be imported by the spec. Node resolves
symlinks for the module URL but leaves argv[1] as given, so reaching the repo
through a symlinked path made the two differ and main() never ran. The script
then exited 0 with no output, the snapshot was never regenerated, `git diff`
came back empty, and the gate reported an unchanged surface. No error, no
warning. On macOS /tmp is a symlink to /private/tmp, which is how the
throwaway worktree used while building this reached the repo.

Splitting the CLI into its own file removes the decision rather than correcting
it — importing the module now does nothing on its own, so there is no condition
left to get wrong. Verified through both a symlinked and a direct path; the
generated file is unchanged.
…pshot

Several public paths map to the same source file, so the snapshot was
byte-identical after one of them was deleted: the declaration stayed
reachable through its sibling. Dropping
'Channel/components/MessageInput' left the gate green while
'@sendbird/uikit-react/Channel/components/MessageInput' started failing
with ERR_PACKAGE_PATH_NOT_EXPORTED for anyone importing it.

The walk only consumed Object.values(moduleExports), discarding the path
names entirely. Emit them as a header ahead of the declarations, sorted
by public path so the block does not move with declaration order.

Covered by five cases that go through renderSnapshot rather than
renderEntryPoints: asserting the pure function leaves them all passing
when the header is unwired from the written file.
`lame.all` is the only entry whose source is plain JavaScript, so the
`.replace(/\.tsx?$/, '.d.ts')` that derives the declaration path silently
left it alone. The published manifest named
types/_externals/lamejs/lame.all.js under both `exports` and
`typesVersions`, and tsc --emitDeclarationOnly never writes that file:
publint reports it as an error and the directory is absent from the
tarball.

Omit the `types` condition when the source emits no declaration. Against
the manifest published as 3.19.0 the only differences are the dropped
key and its typesVersions entry; the other 203 export entries are
unchanged. attw moves the subpath from NoResolution to
UntypedResolution, so consumers go from finding nothing to finding the
JavaScript without types.

Extract the map into its own module so the shape can be asserted without
running the build.
movePackageJSON() was called without await, so its write raced the
synchronous work that followed it. execSync blocks the loop until it
returns, which means a failure in buildTypeDefinitions() ends the
process before the continuation ever runs and dist/package.json is left
missing. Observed while running the script with tsc off PATH: exit 127,
no manifest.

Sequence the steps and report a failure explicitly rather than as an
unhandled rejection.
The surface snapshot proves the declarations did not change. It says
nothing about whether a consumer can reach them: it walks dist/types
from the entry sources, so an exports target that resolves nowhere is
absent from the snapshot instead of visible in it.

Add publint and attw over dist. Known-failing categories are excluded
rather than fixed here, so the gate reports regressions only:

- false-esm: `dist/package.json` declares "type": "module" while
  `dist/cjs` declares commonjs, so a node16/nodenext consumer emitting
  CommonJS gets TS1479 on every entry. bundler, legacy node and ESM
  consumers all typecheck. Worth its own change: correcting it means
  emitting a second set of declarations for the require condition across
  all 204 entries.
- internal-resolution-error: relative specifiers inside the declaration
  files. A consumer with skipLibCheck disabled reports none of them, so
  this is not reachable from outside the package.

Both entries excluded by path are untyped by nature: the stylesheet and
lame.all.
c661f806 ordered the steps so dist/package.json is written before
anything that can throw, but nothing held that ordering: the suite
covers the export map alone and stays green when the await is dropped
and tsc fails.

Run the real script in a sandbox rather than against dist/, which the
surface snapshot and package:check both read. A temporary tree gets the
three scripts, a single-entry module map and empty dist directories, and
a stub tsc ahead of PATH makes the declaration step fail the same way on
any machine.

Two assertions, each covering a different way the fix can rot: dropping
the await loses the manifest, and swallowing the rejection loses the
exit code. Neither catches the other, since an unhandled rejection also
exits non-zero.
Nothing read the TypeScript this branch added under scripts/. eslint is
scoped to 'src/**/*.ts*', tsconfig.json includes only src and
vitest-setup.ts, and vitest strips types without checking them: a
`const x: number` assigned a string passed typecheck, lint and test
alike.

A separate project rather than widening tsconfig.json, because the two
options that touch the shared config both fail. Adding "scripts" to
include alone leaves the imports from .js and .mjs typed as any, since
noImplicitAny is off, so the gate reports nothing. Adding allowJs there
reaches the build as well — tsconfig.build.json overrides include but
inherits compilerOptions — and the declaration emit picks up five mock
files under src that its exclude list does not cover, putting them in
the published package.

Scoped this way the emit is untouched at 546 declarations, and a type
error planted in each of the three specs now fails yarn typecheck.

The spec change is a cast: package_exports.js assigns its export entries
through a computed key, which leaves Object.values inferring string[].
@upwind-code-us

upwind-code-us Bot commented Sep 18, 2026

Copy link
Copy Markdown

Upwind Upwind Code Scan - ✅ Passed

0 newly introduced vulnerabilities · 0 resolved · 28 total in this PR vs main

Total breakdown: 🔴 1 Critical | 🔶 8 High | 🟡 18 Medium | 🟢 1 Low

View full analysis in Upwind Console

Scan completed in 9s

Scan history (6 scans)
Commit Scanned at New Resolved Net
001d7b3 2026-09-18 03:52 UTC 0 0 0
25825b3 2026-09-23 00:59 UTC 0 0 0
21d535c 2026-09-23 02:29 UTC +1 -1 0
43e73fe 2026-09-23 05:40 UTC +1 0 +1
d47b0d4 2026-09-23 07:25 UTC +1 0 +1
b4f3df6 < 2026-09-23 08:26 UTC 0 0 0

Last scanned: b4f3df6 · 2026-09-23 08:26 UTC

@netlify

netlify Bot commented Sep 18, 2026

Copy link
Copy Markdown

Deploy Preview for sendbird-uikit-react ready!

Name Link
🔨 Latest commit b4f3df6
🔍 Latest deploy log https://app.netlify.com/projects/sendbird-uikit-react/deploys/6ab38cb4f0f49200084bef44
😎 Deploy Preview https://deploy-preview-1471--sendbird-uikit-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@upwind-code-us

upwind-code-us Bot commented Sep 18, 2026

Copy link
Copy Markdown

Upwind Upwind IaC Scan - ✅ Passed

0 newly introduced misconfigurations · 0 resolved · 0 total in this PR vs main

View full analysis in Upwind Console →

Scan completed in 3s

Scan history (6 scans)
Commit Scanned at New Resolved Net
001d7b3 2026-09-18 03:52 UTC 0 0 0
25825b3 2026-09-23 00:59 UTC 0 0 0
21d535c 2026-09-23 02:29 UTC 0 0 0
43e73fe 2026-09-23 05:40 UTC 0 0 0
d47b0d4 2026-09-23 07:24 UTC 0 0 0
b4f3df6 < 2026-09-23 08:26 UTC 0 0 0

Last scanned: b4f3df6 · 2026-09-23 08:26 UTC

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-23T08:39:22.069041Z b4f3df6 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Comment thread scripts/api_surface.mjs Outdated
return (
header +
declarations
.map((file) => `// ===== ${relative(typesDir, file)} =====\n${readFileSync(file, 'utf-8')}`)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

파일을 통째로 담으면 공개 표면과 무관한 변경까지 스냅샷이 흔들립니다. export 하지 않은 내부 타입 수정이나 import 경로 변경 같은 것들인데, 커밋된 7,228줄 중 import 문만 814줄입니다. 내부 파일 하나를 옮겨도 CI 가 "공개 API 표면이 바뀌었다"로 실패하게 됩니다.

파일 단위보다 export 된 심볼 단위로 비교하는 쪽이 좋겠습니다.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

진단이 맞았습니다. 수치도 그대로 확인했습니다 — 7,228줄 중 import가 814줄이고, 내부 파일 하나를 옮기면 경로만 바뀌는데 CI가 "공개 API 표면이 바뀌었다"로 떨어집니다.

내부 선언은 심볼 단위로 바꿨습니다. 파일 경로는 더 이상 키에 들어가지 않고, 참조도 대상 파일이 아니라 그 심볼을 가리킵니다. 그래서 어떤 파일에 무관한 export가 하나 늘어도 그 파일을 참조하던 곳이 움직이지 않습니다. 실제로 src/를 고치고 빌드해서 다시 측정한 값입니다.

바꾼 것 스냅샷
내부 파일 이름 변경 0줄
내부 시그니처 한 줄 변경 2줄 (그 줄만)
공개 export 추가 1줄
공개 subpath가 이름 하나를 잃음 2줄, 전부 -

다만 진입점은 파일 단위로 남겼습니다. 어떤 subpath가 어떤 이름을 내보내는지가 그 자체로 계약이기 때문입니다. 위 마지막 행이 그 경우인데, "이 심볼이 트리 어딘가에 존재한다"만 기록하는 스냅샷에는 비교할 대상이 없습니다 — 선언은 그대로 있고 그 subpath가 더 이상 내보내지 않을 뿐이라서요. 여기에 더해 같은 이름을 서로 다른 선언이 쓰는 경우가 21개 있어, 이름만으로는 어느 쪽이 바뀐 건지도 가려지지 않습니다.

이 선택에는 대가가 있고, 그건 남겨 뒀습니다. 선언을 공개 모듈 밖으로 옮기면 re-export로 계약을 유지하더라도 그 블록이 다시 찍힙니다. subpath가 이름을 잃은 것을 볼 수 있게 해주는 성질과 같은 것이라 둘을 떼어낼 수가 없었습니다. 리뷰어가 "이 이동이 맞는지" 한 번 보게 되는 쪽이 낫다고 판단했습니다.

생성기에는 테스트를 붙였습니다. 단위 테스트 112개와, 별도로 yarn api:conformance 48개입니다. 뒤쪽은 실제 컴파일러로 선언을 만들고, 소스를 바꾼 뒤 소비자 코드를 타입체크해서 소비자가 깨지는 것과 스냅샷이 움직이는 것이 서로 맞는지를 단언합니다. 놓치는 쪽과 헛짚는 쪽을 한 번에 보려고 그렇게 했습니다. 스냅샷은 어느 환경에서 돌려도 같은 바이트가 나옵니다.

감사합니다.

Recording each declaration file verbatim under its path meant the
snapshot moved whenever a path did. Renaming one internal helper rewrote
seven lines — a path header and six import specifiers — with no
declaration body touched, and the build failed with "Public API surface
changed". Moving that file across directories churned 46. A gate that
fails on changes it has no opinion about is one reviewers learn to skim,
and the worth of this file is that an empty diff settles the question.

Internal declarations are now emitted one block per symbol, keyed by the
symbol; several declarations sharing a name in one file are one symbol,
as the const-plus-type pair behind an enum is. Entry points keep whole-
file blocks, keyed by the public subpath: which symbols a public module
exports is itself the contract, and discarding that is what makes a
purely symbol-keyed snapshot unsafe. Move a type out of
GroupChannel/components/TypingIndicator without re-exporting it and the
subpath stops exporting it, while a snapshot that only records "this
symbol exists somewhere" reports nothing. Here it moves 10 lines.

A symbol is identified by what it declares and by what that declaration
refers to — imported types and types declared beside it alike — resolved
to a fixed point that ends when the keys stop moving rather than after a
set number of passes. A chain of aliases is as long as someone wrote it,
and a difference at its far end still has to reach the name a public
path exports; each pass re-renders only the symbols whose references
moved — tracked as the keys a rendering actually read, not scraped back
out of its text, so that a reference wearing a type-only marker or
sitting inside a namespace still counts, and carried along when a second
declaration reuses a lookup the first one already made — which is what
keeps a five-hundred-deep chain to under a second instead of forty. Two
interfaces that read identically but reach for different types are told
apart, and a reference carries the same key its block does.

A reference names the symbol it reaches for rather than the file that
happens to hold it, so an unrelated export joining that file moves
nothing. Names are followed to their declaration through aliases,
through a binding that was imported and then exported on, through a
namespace, and out to a package, whose specifier is kept since that is
as far as this repository can see. What a module publishes a name as and
what it calls that name inside itself belong to two namespaces, and each
is followed on its own terms: an export name through the module's export
list, its explicit re-exports and its stars; a local name through its
declarations and its imports; the first handing to the second where an
export list renames on the way out. One lookup answering for both is
wrong in both directions — a module that publishes one local name under
another's sends that other name to the wrong declaration, and a name
reaches a declaration through an `import type` the module never
exported, which is where the ChannelSettings entry had been picking up a
type-only marker it has no claim to. `default` is an export name like
the rest, reached through a rename, a re-export, or a plain `export
default`, rather than by matching the text of the last. A walk after one
name records the name it was after at each module it passed and not the
module alone, in both namespaces: arriving at a file a second time for a
different export is how a pair of barrels that hand names to each other
reads, and calling that a circle abandons the walk one hop short of the
declaration. Whether a name leaves type-only
travels with it, because that is the difference between a consumer
constructing a class and failing to.

Two forms point at the module because the module is what they mean —
`export *` and `import * as` — and a module is keyed by what it exports:
every exported name, the symbol it resolves to, and whether it leaves
type-only. A star is spread before anything written out beside it — a
declaration, a local export list, an explicit re-export — so those win
the way TypeScript gives them, both when a module's exports are read and
when a single name is chased to its declaration. What it spreads is
every exported name but `default`, which the language does not forward:
take away the explicit default re-export standing beside a star and the
module has stopped offering a default. A namespace a module imports and
then exports means the module it was taken from, whether it goes out
under that name or another. `export * as` publishes
one name rather than spreading a module, and is recorded that way; every
star that leaves for a package keeps the package it left for, including
when a barrel passes several of them on at once. Modules that re-export
each other as namespaces would otherwise walk in a circle, so each walk
remembers where it has been — down its own path, so that two branches
reaching the same module are not mistaken for a loop, and through a
named re-export, which is another way into the same circle. Breaking the
circle keeps which module it turned back to, since turning back to one
module is not the same contract as turning back to another — by where it
sits on the path, so that modules which declare nothing of their own and
only pass names through are still told apart. A result reached that way
belongs to the path that found it, so it stays out of the cache rather
than being handed to the next reader, whose path may differ and whose
file names have nothing to do with the contract, which holds for a
circle of plain stars as much as for one of namespaces. An internal
barrel is otherwise free to drop a name, rename one, narrow one to a
type or repoint one elsewhere while the snapshot stays still.

A block carries the references that bind the names it uses, and only
those. Every import is split one name per line — package imports
included, their specifiers left alone since they name another package's
contract — and the set is computed from every local binding a statement
introduces, named, aliased, default and namespace alike. Attaching a
whole statement instead would pull in names the declaration never
mentions, which is enough to give one shared declaration two different
bodies and split it into two blocks.

Checked against real builds rather than edited declarations. Five
reorganisations that leave the public surface alone produce no diff at
all: rename, directory move, split behind a barrel, moving a declaration
between internal modules, and moving an entry's source while its subpath
stays. Five real changes still show — a changed internal signature (2
lines), a new public export (1), an entry losing a symbol (10), an entry
removed (63), and a prop added to three same-named public types (3). Two
lines is what a symbol whose name is its own alone costs; where a name is
shared, its key carries a digest of what the symbol declares, so a
one-line change moves the key along with the body and reads as twelve.

The entry block draws the boundary. Moving a declaration out of a public
module shows at 11 lines even when a re-export keeps the subpath
exporting it, because the block records that module verbatim. That is
the same property that lets the snapshot see a subpath lose a symbol, so
the two come together; resolving re-exports into the block instead would
expand every entry's closure into it.

Nothing is dropped in the move: every declaration line of all 294
collected files is present, along with all 725 exported names, 93
@deprecated tags, and 201 of the 202 public subpaths — the one left out
is vendored JavaScript, which the compiler emits no declarations for and
so has no surface to record. All 730 block keys are unique, every
reference either names a block, a module, or a package, and the file
regenerates byte-identically in under two seconds — on any machine.
Blocks are laid out by comparing their keys as strings rather than by a
collation, which changes with the shell's locale: under a
Turkish one the old comparator moved six lines on its own, and a gate
read by `git diff --quiet` would have called that a changed public
surface.

Symbol-level extraction can shrink silently — a declaration the splitter
fails to recognise leaves no trace, and a smaller snapshot reads as a
clean surface. The spec asserts every line of every collected file
survives, that keys stay unique, that a block keeps the reference
binding each name it uses and none it does not, and that a reference
does not move when the file behind it gains an unrelated export. It
covers the shapes that are easy to drop or to over-collect: a
declaration behind a doc comment, a file-level comment ahead of the
imports, a module's local export list, a side-effect import with no name
to attach it to, a binding that is not the first name in its import, a
default and named import sharing one statement, and the const-and-type
pair one name can stand for, where keeping only the declaration written
last drops the other without narrowing anything the snapshot reports.
Each was confirmed by reverting the fix and watching the suite go red.
The snapshot spec writes its declaration fixtures by hand, which means
it can only assert what the person writing it already believed. A
fixture can be a rung shallower than the shape it is standing in for,
and then reverting the fix under test leaves the suite green — which
happened while this generator was being reviewed. A fixture that is
wrong in the same direction as the code proves nothing.

`yarn api:conformance` derives both ends from tools instead. Each case
is a small package of TypeScript sources: the real compiler emits the
declarations, the generator snapshots them, the sources change, and the
compiler then type-checks a consumer against the rebuilt package. The
case passes when the consumer breaking and the snapshot moving agree — a
change a consumer feels has to show, and one it does not feel must not.
The second half is the half the spec was weakest on, and it is what
caught a file rename turning into a reported API change.

Thirty-eight cases, each one a shape that slipped past the generator at
some point: a name narrowed to type-only along a re-export chain, a
binding imported and then exported on under another name, an export list
whose public names collide with the local ones it draws from, a
namespace renamed, repointed, or published by name, a re-export leaving
for a different package, a star asked to carry the default export behind
it, a default import landing on a module that only re-exports one, a
chain that passes back through a module it has already been to for a
different name, one name standing for both a const and a type, two
declarations that read alike but reach for different types, modules that
re-export each other in a circle. `--deep` adds a five-hundred-link
alias chain, which takes about a minute on its own.

A case can also pass while standing for nothing, so the runner refuses
the three ways that happens: a change naming a file the case never
compiles, a change rewriting a file with the text it already had, and a
build whose emit the compiler skipped — the last of which would leave
the previous build in place and snapshot it a second time.

Kept out of `yarn test`, which it would roughly double. This is the
suite to run when `scripts/api_surface.mjs` changes, and reverting any
of that file's handling of stars, cycles, aliases, namespaces or type-
only exports fails it.
@sf-tyler-jeong

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 25825b3afe

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/api_surface.mjs Outdated
A module written for Node's ESM resolution names its neighbour with the
extension that neighbour will have at runtime — `export { Foo } from
'./foo.js'` — and declaration emit keeps the specifier as written while
emitting the target as `foo.d.ts`. Probing for `foo.js.d.ts` and
`foo.js/index.d.ts` finds neither, so the target never joins the
collected set and the snapshot holds no record of what `Foo` is:
changing it leaves the file byte-identical while a consumer stops
compiling. A gate that reports nothing is worse than no gate, because an
empty diff is read as an answer.

The runtime extension is now substituted the way the compiler does it
before the candidates are probed, and the substituted form is tried
first: `.js` and `.jsx` stand for a `.d.ts`, `.mjs` and `.cjs` for a
`.d.mts` and a `.d.cts`. Where none of those exist the specifier is read
literally, again as the compiler reads it — `./foo.cjs` beside both a
`foo.ts` and a `foo.cjs.ts` means the second, and letting a bare `.d.ts`
stand in for the missing `.d.cts` would answer with the first. A
specifier that names a declaration file outright is taken as it stands:
`import type { Foo } from './foo.d.ts'` is what the compiler suggests
when it turns the plain import down, it type checks, and declaration
emit keeps it word for word. Nothing in this package spells an extension
out today, so the snapshot does not move — the point is that being right
about it costs nothing before something does.

Checked by resolving every arrangement of the files a specifier could
mean, 112 of them, against the compiler's own answer.

Covered by two conformance cases, where the compiler emits the
declarations, the type behind the specifier changes, and the consumer
and the snapshot have to agree, and by three tests over the resolver
itself, since `.jsx` needs a compiler flag the conformance fixtures do
not set and a declaration file is not re-emitted into an output
directory at all. Dropping the substitution fails two of the tests and
one of the cases; keeping it but letting the bare `.d.ts` stand in fails
one of each; taking the declaration extension away fails one test and
puts four arrangements back out of step with the compiler.
@sf-tyler-jeong

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21d535c213

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/api_surface.mjs
Comment thread scripts/api_surface.mjs Outdated
Two more ways a declaration went unread. The snapshot is only as good as
the set of files it reaches, and a file it never reaches is a change it
can never report — the diff comes back empty, and an empty diff is read
as an answer.

A triple-slash `<reference path="...">` names a declaration the way an
import specifier does, and a public signature can rest on a global it
brings in. Only `from` and `import()` were being scanned, so the
referenced file stayed outside the collected set and whatever it
declared could change unremarked. Reference paths are followed now
alongside the specifiers, and it is the compiler's own directive parser
that reads them: matching the shape by hand went wrong in six of
eighteen spellings, once by missing a file the compiler reads — an
attribute name is not case sensitive — and five times by collecting one
it does not, among them a directive with no closing slash and a
directive quoted inside a string.

A default export carries the name of whatever it declares, so an
interface as well as a function or a class — those three and no others,
since an enum or a namespace cannot be default-exported at all. Reading
`export default interface Shape` as anonymous gives two modules that
default-export different shapes the
same export signature, and so the same module key: point an entry at the
other one and nothing moves while a consumer stops compiling. This
package already has such a module, under `modules/App`, though nothing
reaches it by module key today.

Neither changes the snapshot; reading the directives costs the run about
a third of a second, spent once. Covered by two conformance cases, where
the compiler emits the declarations, the type behind the reference
changes, and the consumer and the snapshot have to agree, and by three
tests over the walk itself, one of them holding the directive grammar to
what the compiler accepts. Reverting either fix fails one case and one
test; the default one has to swap which module an entry points at rather
than edit a type in place, since editing one rewrites its block and
would pass whether the module key were right or not.
The test that carries a difference down a 520-link alias chain takes
about 1.9s on its own and 5.5s when the whole suite is running beside
it, which is over vitest's 5s default. It passes when run alone and
fails when run with everything else, which is the shape of a test that
fails in CI and not on the machine of whoever is asked to look at it.

The length is the point — the chain has to be longer than any fixed
number of passes the generator could make — so the budget is what gives
rather than the fixture. Thirty seconds leaves room for a loaded runner
without letting a genuine hang sit there.
@sf-tyler-jeong

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 43e73fe8e9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/api_surface.mjs Outdated
One statement can bind several names — `export declare const foo = "",
bar = 1;` is what declaration emit gives back for a source that wrote
them together — and only the first was being recorded. The second
belonged to no symbol, so a reference to it resolved to nothing and
every module that declared a name by that spelling folded to the same
key. Point a public entry at a different one of them and the snapshot
holds still while a consumer stops compiling. `const enum E` went the
same way for a different reason: the reader took `const` for the whole
of the declaration and called the thing `enum`.

Every name a statement binds is recorded now, against the statement they
share. Which names a declaration refers to is then read from the shape
of the block rather than from its text, because an identifier sitting
where a name is being declared refers to nothing, while the same
identifier in a type position refers to something. Both halves of that
matter and they pull opposite ways: a value can be typed by a type of
the same name declared beside it, and losing that reference makes two
unlike modules read alike, while counting the name written beside it
makes two names depend on each other, and then an unrelated module
leaving the tree moves both of their keys. The exception is a property
keyed by a name rather than labelled with one — `[KEY]: string` is a
reference to the `KEY` declared above it, and that subtree is read
rather than skipped with the rest of the name.

The names are read with the compiler's parser rather than by splitting
on commas, because the commas that matter are not the only ones on the
line: `Map<string, number>` and `(a: string, b: number) => void` each
carry one that binds nothing. Only statements that open with `const`,
`let` or `var` are parsed for their names — everything else takes its
name from the declaration line, as before — so the run is no slower.

One block moves: a `_default` whose property happens to carry the name
of a declaration in the same file was depending on it, though the
property's type is written out in full and never names it. Changing that
declaration still moves the block, through its body. Nothing else here
writes two names into one statement or declares a `const enum`, so the
rest of the snapshot stands.

Covered by six conformance cases that each hold a public entry still
while the modules behind it move, and by eight tests over the reader,
one of them holding the commas inside a type. Recording only the first
name again fails two cases and three tests; reading the const enum by
the line fails one of each; leaving out every name written beside the
one being keyed fails one of each; taking the declaration sites for
references fails one case and two tests; and skipping a computed
property name with the rest of the name fails two cases and one test.
@sf-tyler-jeong

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d47b0d4174

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/api_surface.mjs
Comment thread scripts/api_surface.mjs Outdated
Two more ways the walk and the module signature lost track of something a
consumer can feel.

A side-effect import names a declaration as surely as one with a `from`
clause. `import './augment';` is how a module brings in a global or an
augmentation, and a public signature can rest on what it declares, but
only specifiers with a `from` or an `import()` were being scanned, so the
file stayed outside the collected set and whatever it declared could
change unremarked. The compiler's preprocessor already reads that file
for its reference directives; its list of imported files is read along
with them now, and the relative ones are followed.

A module that default-exports something without naming it —
`export default class { ... }` — had that recorded as the word
`anonymous`, so two such modules had the same export signature and the
same module key. Point a public alias at the other one and nothing moves
while a consumer's type changes underneath. The declaration is in the
snapshot already, under a key derived from what it says; the signature
carries that key now instead of a word that means nothing.

Neither changes the snapshot: nothing here imports a declaration for its
side effect — all 114 bare relative imports in the collected declarations
are stylesheets — and nothing default-exports without a name. Covered by
a conformance case each, both of which swap which of two modules a public
entry reaches for, and by a test each over the walk and the snapshot.
Reverting either fails one case and one test.
@sf-tyler-jeong

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b4f3df6a07

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/api_surface.mjs

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants